Skip to content

xdsclient: fix panic on empty resource in ADS response#8970

Merged
Pranjali-2501 merged 6 commits intogrpc:masterfrom
Pranjali-2501:bug
Mar 17, 2026
Merged

xdsclient: fix panic on empty resource in ADS response#8970
Pranjali-2501 merged 6 commits intogrpc:masterfrom
Pranjali-2501:bug

Conversation

@Pranjali-2501
Copy link
Copy Markdown
Contributor

@Pranjali-2501 Pranjali-2501 commented Mar 10, 2026 •
edited by arjan-bal
Loading

This PR fixes an intermittent panic in xdsclient decodeResponse that occurs when processing an xDS ADS response containing an empty resource name, while doing fuzz testing.

RELEASE NOTES:

  • xds: Fix a panic in the xDS client that can occur when processing an ADS response containing an empty resource name.

@Pranjali-2501 Pranjali-2501 added this to the 1.81 Release milestone Mar 10, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 10, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 81.25000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.11%. Comparing base (81c7924) to head (054d10a).
⚠️ Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
internal/xds/clients/xdsclient/channel.go 62.50% 2 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #8970      +/-   ##
==========================================
+ Coverage   82.67%   83.11%   +0.43%     
==========================================
  Files         410      411       +1     
  Lines       32573    32715     +142     
==========================================
+ Hits        26929    27190     +261     
- Misses       4055     4149      +94     
+ Partials     1589     1376     -213
Files with missing lines Coverage Δ
...nternal/xds/xdsclient/xdsresource/unmarshal_cds.go 88.70% <100.00%> (+0.36%) ⬆️
...nternal/xds/xdsclient/xdsresource/unmarshal_eds.go 96.20% <100.00%> (+0.04%) ⬆️
...nternal/xds/xdsclient/xdsresource/unmarshal_lds.go 93.41% <100.00%> (+0.03%) ⬆️
...nternal/xds/xdsclient/xdsresource/unmarshal_rds.go 87.84% <100.00%> (+0.09%) ⬆️
internal/xds/clients/xdsclient/channel.go 78.67% <62.50%> (-3.28%) ⬇️

... and 43 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Pranjali-2501 Pranjali-2501 requested a review from arjan-bal March 10, 2026 16:13
arjan-bal
arjan-bal reviewed Mar 10, 2026
View reviewed changes
@arjan-bal arjan-bal assigned Pranjali-2501 and unassigned arjan-bal Mar 10, 2026
@Pranjali-2501 Pranjali-2501 requested a review from arjan-bal March 13, 2026 05:33
arjan-bal
arjan-bal reviewed Mar 13, 2026
View reviewed changes
@arjan-bal arjan-bal assigned Pranjali-2501 and unassigned arjan-bal Mar 13, 2026
@arjan-bal arjan-bal added the Area: xDS Includes everything xDS related, including LB policies used with xDS. label Mar 13, 2026
@arjan-bal arjan-bal assigned Pranjali-2501 and unassigned arjan-bal Mar 13, 2026
@arjan-bal
Copy link
Copy Markdown
Contributor

arjan-bal commented Mar 13, 2026

Please also add a release note mentioning the bug fix.

easwars
easwars reviewed Mar 15, 2026
View reviewed changes
@Pranjali-2501 Pranjali-2501 requested a review from easwars March 16, 2026 12:24
@easwars
Copy link
Copy Markdown
Contributor

easwars commented Mar 16, 2026

I just happened to look at this PR by chance. Since @arjan-bal is already reviewing it, I'll leave it to him. I don't want both of us spending time on the PR. Thanks.

@easwars easwars removed their request for review March 16, 2026 16:38
@Pranjali-2501 Pranjali-2501 requested a review from arjan-bal March 16, 2026 18:34
@arjan-bal
Copy link
Copy Markdown
Contributor

arjan-bal commented Mar 17, 2026

/gemini review

gemini-code-assist[bot]
gemini-code-assist bot reviewed Mar 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses a panic that can occur when processing an xDS ADS response with an empty resource name. The core of the fix is the addition of validation checks in the unmarshalling functions for CDS, EDS, LDS, and RDS resources to ensure resource names are not empty. A defensive check has also been added in decodeResponse to handle cases where a resource decoder might incorrectly return a nil result and nil error, preventing a potential panic. The decodeResponse function has been refactored into a method on xdsChannel for better encapsulation. All related tests have been updated to reflect these changes, including new test cases for empty resource names, ensuring the fix is well-covered. The changes are correct and improve the robustness of the xdsclient.

arjan-bal
arjan-bal approved these changes Mar 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@arjan-bal arjan-bal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@arjan-bal arjan-bal assigned Pranjali-2501 and unassigned arjan-bal Mar 17, 2026
@Pranjali-2501 Pranjali-2501 merged commit 7f78342 into grpc:master Mar 17, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@easwars easwars easwars left review comments

@arjan-bal arjan-bal arjan-bal approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@Pranjali-2501 Pranjali-2501

Labels

Area: xDS Includes everything xDS related, including LB policies used with xDS. Type: Bug

Projects

None yet

Milestone

1.81 Release

Development

Successfully merging this pull request may close these issues.

3 participants

@Pranjali-2501 @arjan-bal @easwars